Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

support nacos metadata #4025

Merged
merged 14 commits into from
May 14, 2019
Merged

support nacos metadata #4025

merged 14 commits into from
May 14, 2019

Conversation

Moriadry-zz
Copy link

What is the purpose of the change

complete nacos metadata report implement.

Brief changelog

nacos metadata report impl, and code clean (switch to using a nacos utils class)

Verifying this change

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a GITHUB_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Run mvn clean install -DskipTests=false & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
  • If this contribution is large, please follow the Software Donation Guide.

@Moriadry-zz
Copy link
Author

@ralf0131 could pls help me check this? I am wondering whether i miss something. Thanks!

@ralf0131 ralf0131 added this to the 2.7.2 milestone May 12, 2019
@ralf0131
Copy link
Contributor

Hi, I am starting to look at this.

@ralf0131
Copy link
Contributor

You need to add <module>dubbo-metadata-report-nacos</module> to dubbo-metadata-report/pom.xml...

@codecov-io
Copy link

codecov-io commented May 13, 2019 •
edited
Loading

Codecov Report

Merging #4025 into master will decrease coverage by 0.09%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #4025     +/-   ##
===========================================
- Coverage     63.11%   63.01%   -0.1%     
- Complexity      564      565      +1     
===========================================
  Files           753      755      +2     
  Lines         32411    32462     +51     
  Branches       5143     5147      +4     
===========================================
  Hits          20455    20455             
- Misses         9604     9655     +51     
  Partials       2352     2352
Impacted Files Coverage Δ Complexity Δ
...tadata/store/nacos/NacosMetadataReportFactory.java 0% <0%> (ø) 0 <0> (?)
...ubbo/metadata/store/nacos/NacosMetadataReport.java 0% <0%> (ø) 0 <0> (?)
.../apache/dubbo/qos/protocol/QosProtocolWrapper.java 64.1% <0%> (-17.95%) 0% <0%> (ø)
...bo/rpc/cluster/support/FailbackClusterInvoker.java 67.21% <0%> (-3.28%) 0% <0%> (ø)
...he/dubbo/registry/multicast/MulticastRegistry.java 67.87% <0%> (-1.81%) 0% <0%> (ø)
.../exchange/support/header/HeaderExchangeServer.java 66.98% <0%> (-0.95%) 0% <0%> (ø)
...dubbo/rpc/protocol/dubbo/CallbackServiceCodec.java 80.14% <0%> (-0.74%) 0% <0%> (ø)
...g/apache/dubbo/registry/consul/ConsulRegistry.java 59.6% <0%> (-0.67%) 27% <0%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e690864...53e16b2. Read the comment docs.

@ralf0131
Copy link
Contributor

ralf0131 commented May 13, 2019 •
edited
Loading

Hi, your code has compile errors, please fix it..

[INFO] Compiling 2 source files to ~/work/apache-dubbo/incubator-dubbo/dubbo-metadata-report/dubbo-metadata-report-nacos/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] ~/work/apache-dubbo/incubator-dubbo/dubbo-metadata-report/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java:[40] error: cannot find symbol
  symbol:   static BACKUP_KEY
  location: class Constants
[ERROR] ~/work/apache-dubbo/incubator-dubbo/dubbo-metadata-report/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java:[80,41] error: cannot find symbol
  symbol:   variable BACKUP_KEY
  location: class NacosMetadataReport
[INFO] 2 errors

@Moriadry-zz
Copy link
Author

Moriadry-zz commented May 13, 2019 •
edited
Loading

@ralf0131 yes, fixed it. Sorry for missing this.

@ralf0131
Copy link
Contributor 

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  18:51 min
[INFO] Finished at: 2019-05-13T09:56:44Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.0:testCompile (default-testCompile) on project dubbo-metadata-report-nacos: Compilation failure: Compilation failure: 
[ERROR] /home/travis/build/apache/incubator-dubbo/dubbo-metadata-report/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReportTest.java:[47,39] error: cannot find symbol
[ERROR]   symbol:   variable SESSION_TIMEOUT_KEY
[ERROR]   location: class Constants
[ERROR] /home/travis/build/apache/incubator-dubbo/dubbo-metadata-report/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReportTest.java:[92,79] error: cannot find symbol
[ERROR]   symbol:   variable PROVIDER_SIDE
[ERROR]   location: class Constants
[ERROR] /home/travis/build/apache/incubator-dubbo/dubbo-metadata-report/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReportTest.java:[104,111] error: cannot find symbol
[ERROR]   symbol:   variable CONSUMER_SIDE
[ERROR]   location: class Constants
[ERROR] -> [Help 1]

@Moriadry-zz
Copy link
Author

Moriadry-zz commented May 13, 2019 •
edited
Loading

@ralf0131 hi, I merged with remote/master, can you check again?

@ralf0131 ralf0131 mentioned this pull request May 14, 2019
Merged
6 tasks
ralf0131
ralf0131 approved these changes May 14, 2019
View reviewed changes
Copy link
Contributor

@ralf0131 ralf0131 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I tested locally. The metadata can be published to Nacos successfully. Unit test passed locally as well.

@ralf0131
Copy link
Contributor

This pull request is targeted to solve #3846

@ralf0131 ralf0131 merged commit d77a009 into apache:master May 14, 2019
@Moriadry-zz Moriadry-zz deleted the feature/nacos-metadata branch May 14, 2019 03:58
@dubbo-bot dubbo-bot mentioned this pull request May 17, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ralf0131 ralf0131 ralf0131 approved these changes

Assignees

@Moriadry-zz Moriadry-zz

Labels
None yet
Projects
None yet
Milestone
2.7.2
Development

Successfully merging this pull request may close these issues.
4 participants
@Moriadry-zz @ralf0131 @codecov-io @moriadry